-
Notifications
You must be signed in to change notification settings - Fork 95
AI models encoder update #1302
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
AI models encoder update #1302
Conversation
Encoder is updated to handle null values The models are applied on three SUT 1. NCS API from EMB 2. Basic API 3. Multitype API
Encoder is updated to handle null values The models are applied on three SUT 1. NCS API from EMB examples 2. Basic API 3. Multitype API
The encoder has been updated to handle null values. The models are applied to the SUTs Basic API Multitype API
@@ -36,18 +36,37 @@ class GaussianOnlineClassifier : AIModel { | |||
private var density200: Density? = null | |||
private var density400: Density? = null | |||
|
|||
// classifier state | |||
private var cp: Int = 0 // correct predictions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 things:
- when describing fields, do not use //, but rather JavaDoc style /** */ (note the first 2 *). this is because documentation is generated, and will be visible when howevering over the variable with mouse in an IDE like IntelliJ
- do not us acronyms for object level fields. it makes the code harder to read. short acronyms make sense when are for local variables inside a block with short lifespan. the above can be rewritten in:
private var correctPredictions: Int = 0
. note that IntelliJ will autocomplete variable when you start typing them
fun setDimension(d: Int) { | ||
require(d > 0) { "Dimension must be positive." } | ||
this.dimension = d | ||
this.density200 = Density(d) | ||
this.density400 = Density(d) | ||
this.cp = 0 | ||
this.tot = 1 | ||
this.ac = 0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as this can be useful by different models, shouldn't be abstracted into its own class, eg something like ModelAccuracy
, so it can be re-used in the different model implementations?
} | ||
|
||
fun getDimension(): Int { | ||
check(this.dimension != null) { "Classifier not initialized. Call setDimension first." } | ||
return this.dimension!! | ||
} | ||
|
||
fun setCp(cp: Int) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these setters and getters are redundant in Kotlin, as automatically generated by compiler if you declare it like var correctPredictions: Int = 0
(note the not use of private
here, as it seems you need get and set both to be public). have a read at: https://kotlinlang.org/docs/properties.html#custom-getters-and-setters
|
||
fun getCorrectPredictions(): Int = cp | ||
fun getTotalRequests(): Int = tot | ||
fun getAccuracy(): Double = if (tot == 0) 0.0 else cp.toDouble() / tot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you keep a variable for ac
but then recompute it on the fly here? or is the field ac
redundant? note that this logic can integrated inside a ModelAccuracy
to enable reuse
if (this.tot == 0) { | ||
return 0.0 | ||
} | ||
return this.cp.toDouble() / this.tot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the difference of this with getAccuracy
? shouldn't this method just call instead of duplicating the logic?
// classifier state | ||
private var cp: Int = 0 // correct predictions | ||
private var tot: Int = 1 // total requests | ||
private var ac: Double = 0.0 // accuracy ratio |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you are using exactly the same code as in another class, that is good hint for 2 possible solutions: (1) a shared parent class in which the common logic is pushed up; (2) or extract the functionality in a new class instantiated in both places (eg ModelAccuracy
)
|
||
fun getCorrectPredictions(): Int = cp | ||
fun getTotalRequests(): Int = tot | ||
fun getAccuracy(): Double = if (tot == 0) 0.0 else cp.toDouble() / tot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
duplicated code should be avoiaded.
@@ -1,239 +0,0 @@ | |||
package org.evomaster.core.output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why deleting the content of this test???
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I have no idea. Could it be because of the manual merge strategy that I chose?
Anyway, sorry about that.
The encoder has been updated to handle null values.
The models are applied to the SUTs
The results are promising.
Please run
AIGaussianCheck.kt
andAIGLMCheck.kt
to see the performance.